Add a host-only path for refreshing the Kotlin bindings - #475
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughThe PR adds a strict host-based script for generating Kotlin UniFFI bindings. It validates the build, locates the host library, verifies generated output, and documents that the result supports typechecking but not runtime JNI execution. ChangesKotlin binding refresh
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant refresh_bindings_sh
participant Cargo
participant UniFFI_bindgen
participant Kotlin_bindings
refresh_bindings_sh->>Cargo: host-build keep-mobile
Cargo-->>refresh_bindings_sh: platform-specific cdylib
refresh_bindings_sh->>UniFFI_bindgen: generate Kotlin bindings from cdylib
UniFFI_bindgen-->>Kotlin_bindings: generated Kotlin source
refresh_bindings_sh->>Kotlin_bindings: apply suppressions and verify output
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Summary
Changing keep-mobile's FFI leaves the generated Kotlin stale, and the only way to refresh it was
build-rust.sh, which cross-compiles for every Android ABI first. That is what a runnable APK needs, but it means the repository cannot be typechecked against a changed core without the NDK, cargo-ndk and the Android targets, even though the Kotlin uniffi emits is platform independent.uniffi reads interface metadata from the library rather than machine code, so a host build carries the same metadata as a cross-compiled one. The new script builds for the host and generates from that.
It is deliberately not a replacement. No
jniLibsare written, so the app compiles and then fails at runtime with anUnsatisfiedLinkError. The script says so on its last line and the README says so where it is introduced, because a faster path that quietly produces something unrunnable would cost more time than it saves.Two details worth noting. The target directory is resolved through
cargo metadatarather than assumed to be../target, since a workspace may setCARGO_TARGET_DIRand guessing would report a missing library that had in fact just built. And the same suppression rewritebuild-rust.shperforms is applied here, so the two paths produce the same file and switching between them does not surface as a diff.Test plan
Verified end to end rather than by inspection. The script was run against this checkout, and the Kotlin it produced is byte-identical to the bindings the cross-compiled path had already written, confirmed with
diff. The suppression line matches../gradlew compileDebugKotlinthen succeeds against the regenerated file, which is the thing the script exists to enable.Shell syntax checked with
bash -n. Both failure paths were written to fail loudly: a missingKEEP_REPO, and a library that is absent after a successful build, which would mean keep-mobile had stopped being a cdylib.Summary by CodeRabbit
New Features
Documentation